fix: guard AddPowderPatternDiffraction setup - #94
Conversation
Co-authored-by: Copilot App <[email protected]>
Co-authored-by: Copilot App <[email protected]>
Co-authored-by: Copilot App <[email protected]>
eaadf68 to
be1d825
Compare
Co-authored-by: Copilot App <[email protected]>
Co-authored-by: Copilot App <[email protected]>
Co-authored-by: Copilot App <[email protected]>
|
@vincefn lgtm. Do you want to roll this towards a new release shortly? Let me know if you want me to do anything. For the other codes we are starting to migrate from python |
|
Thanks, I'll merge this. As for migrating for nanobind, I guess we have to, boost starts to show its age. But I have no idea how hard the migration would be (then again with AI maybe all the wrapping and object ownership issues will be handled easily..). There are a few incoming changes notably from the work of @clemisch (working on large powder diffraction maps automated Rietveld/profile fitting) so maybe it can be done after that has been released. I don't see yet c++ changes in the I'm also wondering if we actually need |
|
We have done the nanobind migration (undergoing review) for libdiffpy and it is not too bad. We didn't start on pyobjcryst/libobjcryst but if we decide to retire libobjcryst, this would seem to be a good time to do it, when we do this migration. |
2 participants
Summary
AddPowderPatternDiffraction()so a no-reflections failure does not leave a broken partial component attached to the powder patternnews/94.rstitemRoot cause
addppdiffraction()constructed aPowderPatternDiffraction, attached it to the powder-pattern setup path, and then let the diffraction preparation step raise from the no-reflections case. That left the convenience wrapper able to fail after partially wiring the diffraction object into the pattern lifecycle.Fix
The wrapper now keeps the temporary diffraction component under local ownership during setup, assigns the crystal, prepares it against the target powder-pattern context, and only registers it with the
PowderPatternafter preparation succeeds. This preserves the expectedObjCrystExceptionfor an empty range while preventing the failed component from being retained by the powder pattern.Regression coverage
Local targeted coverage:
PYTHONPATH=src python -m pytest -q src/pyobjcryst/tests/testpowderpattern.py -k 'rollback or update_nbrefl'The new rollback test asserts that
AddPowderPatternDiffraction()raises on a no-reflections range and that the powder pattern still reports zero components afterward. The existingtest_update_nbreflcase keeps a successful diffraction-add path covered in the same targeted run.